Skip to content

improvement(knowledge): maintain keyword projections only for search indexes - #8333

Merged
waleedlatif1 merged 4 commits into
stagingfrom
improvement/scope-keyword-projections-to-search-indexes
Sep 26, 2026
Merged

waleedlatif1 merged 4 commits into
stagingfrom
improvement/scope-keyword-projections-to-search-indexes

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Sep 26, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Since improvement(search): consolidate knowledge search around live Sim Search and document-decided workspace retrieval #8314, workspace knowledge search ranks on embedding/document. The only live reader of embedding_keyword_search and embedding_keyword_tin is dormant indexed search, and only for bases that are search indexes. Yet every chunk write still upserted a keyword row, ran a Tin DELETE on insert, and every document insert fanned an ACL/connector sync out to the projections
  • Script migration 0025_scope_keyword_projections, all CREATE OR REPLACE in one transaction with the 0024 lock-timeout retry:
    • sync_embedding_keyword_search takes the shared membership lock and writes keyword rows only for is_search_index bases. On an update that moves a chunk out of a search index, it deletes the chunk's row
    • A new flip trigger backfills a base's keyword rows when it becomes a search index and deletes them when it stops being one, under the exclusive membership lock. It is separate from the Tin flip trigger, which only exists where Tin is installed. Its upsert skips unchanged rows
    • The Tin chunk trigger skips its DELETE on insert, since a new chunk can't have a Tin row
    • projection_source_acl_sync becomes AFTER UPDATE OF connector_id, acl ... WHEN the value actually changed. A new document has no chunks yet (FK), so the insert arm never matched anything
  • The projector's keyword page writes only for search-index bases, keyed on the base flag
  • Fresh installs, db:push, and late Tin adoption end with the same trigger definitions: 0019 installs the final guarded, key-share-locked Tin triggers, and 0025 re-runs after 0016. A db:push re-run of the legacy 0016 backfill may refill keyword rows for non-search bases; they're unread and left in place like the existing ones
  • Both adoption backfills read their chunks FOR KEY SHARE, so a chunk delete racing an adoption is waited out instead of failing it
  • Rollback floor: v0.9.1 and earlier ranked workspace keyword search through embedding_keyword_search. Once 0025 has run, don't roll back below v0.9.2, and self-hosters should upgrade through v0.9.2 or later
  • Existing keyword rows for non-search bases are left in place (unread) and removed separately

Type of Change

  • Improvement

Testing

  • New 0025_scope_keyword_projections.integration.ts: insert scoping, move-out, adopt and un-adopt, commit and uncommitted-adoption races, no fan-out for inserted documents while a real ACL change still fans out, no-op adoption rewrites skipped, and late Tin adoption keeping the guarded trigger. Each fails with its guard reverted
  • Updated the projector, 0016 backfill, embedding-insert-batches, and processing-lock-scope suites. The search-index fixtures restore real keyword-row assertions
  • Script-migrations integration 68/68, knowledge projection/insert/lock/search suites 44/44, type-check, lint, and check:audits pass

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercel Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
docs Ready Ready Preview Sep 26, 2026 6:52pm UTC

Request Review

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 12 files

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread packages/db/script-migrations/index.ts
Comment thread packages/db/script-migrations/0016_backfill_search_vectors.ts Outdated
Comment thread packages/db/script-migrations/0016_backfill_search_vectors.ts Outdated
Comment thread packages/db/script-migrations/0019_tin_keyword_projection.ts
@greptile-apps

greptile-apps Bot commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Critical risk] Database migration changes keyword projection scope and triggers.

The PR appears safe to merge; no new actionable issue or outstanding previous finding was identified.

Summary

The PR limits keyword projections to search-index knowledge bases and synchronizes projection rows when a base changes type.

  • Adds guarded chunk and membership triggers, updates projector writes, and reconciles migration and db:push paths.
  • Adds integration coverage for adoption, deletion races, and document ACL fan-out.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
  E[Chunk write] --> S{Search-index base?}
  S -- Yes --> K[Write keyword projection]
  S -- No --> N[Skip or remove keyword row]
  F[Base flag changes] --> L[Exclusive membership lock]
  L --> A{Adopted as search index?}
  A -- Yes --> B[Backfill keyword rows]
  A -- No --> D[Delete keyword rows]
Loading

Reviews (3) · Last reviewed commit: "improvement(knowledge): key-share lock t..."

Comment thread packages/db/script-migrations/0016_backfill_search_vectors.ts Outdated
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 12 files

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread packages/db/script-migrations/0025_scope_keyword_projections.ts
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 12 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

@waleedlatif1
waleedlatif1 merged commit 185650f into staging Sep 26, 2026
33 checks passed
@waleedlatif1
waleedlatif1 deleted the improvement/scope-keyword-projections-to-search-indexes branch September 26, 2026 20:12

This branch was successfully deployed

1 active deployment
Preview — decf8220 Deployed Sep 26, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant